Heuristic Improvements with solution hash, MAB and simplex root solution#216
Heuristic Improvements with solution hash, MAB and simplex root solution#216rapids-bot[bot] merged 40 commits intoNVIDIA:branch-25.08from
Conversation
…stic_improvements
|
/ok to test 03ade15 |
aliceb-nv
left a comment
There was a problem hiding this comment.
Looking good :) Just a few questions regarding the hashing
Thank you Akif!
| #if !MIP_INSTANTIATE_FLOAT && !MIP_INSTANTIATE_DOUBLE | ||
| static_assert(false, "MIP_INSTANTIATE_FLOAT or MIP_INSTANTIATE_DOUBLE must be defined"); | ||
| #endif |
There was a problem hiding this comment.
Those atrocious linking errors :) I wish there was a way to throw an error at preprocessor/link time without having to add explicit checks
There was a problem hiding this comment.
Yeah, this is the second time i spend significant amount of time to these issues.
There was a problem hiding this comment.
Should we just remove FLOAT? do we anticipate using it for MIP?
| thrust::gather(solution.handle_ptr->get_thrust_policy(), | ||
| solution.problem_ptr->integer_indices.begin(), | ||
| solution.problem_ptr->integer_indices.end(), | ||
| solution.assignment.begin(), | ||
| integer_assignment.begin()); |
There was a problem hiding this comment.
I think we ought to be careful with the implicit cast to size_t performed here. The C++ standard states that any value that cannot be represented in the target cast type yields undefined behavior:
Real floating-integer conversions
A finite value of any real floating type can be implicitly converted to any integer type. Except where covered by boolean conversion above, the rules are:The fractional part is discarded (truncated towards zero).
If the resulting value can be represented by the target type, that value is used
otherwise, the behavior is undefined.
int n = 3.14; // n == 3
int x = 1e10; // undefined behavior for 32-bit int
I think this mean there might be issues in the case of negative and large (>1e20) assignments. It's probably better to compute the hash in a bitwise manner
There was a problem hiding this comment.
Good catch, fixed!
| hash_1 ^= hash_2 + 0x9e3779b9 + (hash_1 << 6) + (hash_1 >> 2); | ||
| return hash_1; |
There was a problem hiding this comment.
I think this is a 32-bit hash, with 64-bit values this might not work properly
|
/ok to test 6bcd726 |
|
/ok to test 08d8372 |
| solution.problem_ptr->integer_indices.end(), | ||
| solution.assignment.begin(), | ||
| integer_assignment.begin()); | ||
| reinterpret_cast<double*>(integer_assignment.data())); |
There was a problem hiding this comment.
I seriously doubt we will ever use anything but FP64, but in the rare case we instantiate the solver with f_t=FP32, this is going to break/behave badly. We might want to add a static_assert to make sure sizeof(f_t) == sizeof(size_t) in order not to cause issues if users decide to try out FP32
There was a problem hiding this comment.
okay let me add the static assert here.
|
/ok to test a92b9d4 |
|
/merge |
Description
This PR adds improvements to heuristics framework with the following: